-
Notifications
You must be signed in to change notification settings - Fork 151
feat(gameclient): Introduce imgui framework #2127
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
feat(gameclient): Introduce imgui framework #2127
Conversation
Greptile Overview
|
| Filename | Overview |
|---|---|
| Core/Libraries/Source/ImGui/dx8_backend/imgui_impl_dx8.cpp | Custom DirectX 8 backend for ImGui with proper error checking, device state management, and stencil-based clipping implementation |
| Core/Libraries/Source/ImGui/wrapper/ImGuiFrameManager.h | Well-documented wrapper interface with clear separation of concerns comment explaining RenderDrawData is called separately |
| Generals/Code/GameEngine/Source/GameClient/GameClient.cpp | ImGui integrated into game loop with BeginFrame/ShowDemoWindow/EndFrame at correct points, including before early returns |
| GeneralsMD/Code/GameEngine/Source/GameClient/GameClient.cpp | Zero Hour variant with identical ImGui integration pattern to Generals |
| Generals/Code/Libraries/Source/WWVegas/WW3D2/dx8wrapper.cpp | DX8 device lifecycle integration with ImGui initialization, device object management, and render call in End_Scene() |
| GeneralsMD/Code/Libraries/Source/WWVegas/WW3D2/dx8wrapper.cpp | Zero Hour DX8 wrapper with identical ImGui integration to Generals variant |
Sequence Diagram
sequenceDiagram
participant User
participant WndProc
participant DX8Wrapper
participant GameClient
participant ImGuiWin32
participant ImGuiDX8
participant Display
Note over DX8Wrapper: Application Initialization
DX8Wrapper->>DX8Wrapper: Create_Device()
DX8Wrapper->>ImGuiWin32: ImGui_ImplWin32_Init()
DX8Wrapper->>ImGuiDX8: ImGui_ImplDX8_Init()
DX8Wrapper->>ImGuiDX8: CreateDeviceObjects()
Note over User,Display: Main Game Loop
User->>WndProc: Input Events (mouse, keyboard)
WndProc->>ImGuiWin32: ImGui_ImplWin32_WndProcHandler()
ImGuiWin32-->>WndProc: Returns handled status
GameClient->>GameClient: update()
GameClient->>ImGuiDX8: BeginFrame() [NewFrame sequence]
GameClient->>ImGuiWin32: NewFrame()
GameClient->>GameClient: ImGui::ShowDemoWindow()
GameClient->>GameClient: EndFrame() [Render()]
GameClient->>Display: DRAW()
Display->>DX8Wrapper: End_Scene()
DX8Wrapper->>ImGuiDX8: ImGui_ImplDX8_RenderDrawData()
ImGuiDX8->>ImGuiDX8: SetupRenderState()
ImGuiDX8->>DX8Wrapper: DrawIndexedPrimitive()
Note over DX8Wrapper: Device Reset Handling
DX8Wrapper->>ImGuiDX8: InvalidateDeviceObjects()
DX8Wrapper->>DX8Wrapper: Reset D3D Device
DX8Wrapper->>ImGuiDX8: CreateDeviceObjects()
Note over DX8Wrapper: Application Shutdown
DX8Wrapper->>ImGuiDX8: ImGui_ImplDX8_Shutdown()
DX8Wrapper->>ImGuiWin32: ImGui_ImplWin32_Shutdown()
DX8Wrapper->>DX8Wrapper: DestroyContext()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
14 files reviewed, 4 comments
Greptile found no issues!From now on, if a review finishes and we haven't found any issues, we will not post anything, but you can confirm that we reviewed your changes in the status check section. This feature can be toggled off in your Code Review Settings by deselecting "Create a status check for each PR". |
ab35d8a to
742f8be
Compare
|
relates to : #387 and #2084 (comment) |
de59853 to
d7e3765
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
14 files reviewed, 1 comment
d7e3765 to
4e08d7e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
14 files reviewed, 4 comments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
14 files reviewed, 3 comments
86dec82 to
d72b80f
Compare
d579452 to
21171d1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
21 files reviewed, 1 comment
Thanks. i did exactly that but given the large differences between both backends, some braces might slip the net while i am scanning. i hope i got them all now |
after moving link location multiple targets fail to render imgui. this has been fixed in ZH and generals but is yet to be fixed in worldbuilder
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
25 files reviewed, 1 comment
xezon
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
@xezon since it is no longer active only in Debug mode, i think the Debug label no longer fits . what do you think ? |
|
Debug label is ok. It is meant to be compiled out in regular builds for Players. Just because it does not sit behind RTS_DEBUG does not mean it is no Debug. |
xezon
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A bunch of small comments added.
This needs more work.
I tested in Zero Hour.
Observed bugs
- The mouse cursor is alternating between game and desktop cursor when moving the mouse cursor anywhere in game
- The demo Window has no Close (x) button (there is a global imgui option for this to set, I think)
- Boot load screen doesn ot show imgui
Frame Management
I am not fond of the current frame management. It is incoherent.
I made a commit here with a more coherent frame management proposal:
xezon@a382fbf
The idea is to have GameClient::update using a top level BeginFrame and EndFrame pair, and DXWrapper using a lower level BeginFrame and EndFrame pair. They work together. The pair can also be added anywhere else necessary. For example I have seen that ImGui does render during Boot load screen which you can look into as well.
Additionally, the code in GameClient::update can be simplified and robustified with an RAII helper:
xezon@638685c
| # currently only WIN32 DX is supported | ||
| MESSAGE(FATAL_ERROR "Non-Windows platforms currently not supported for ImGui") | ||
| endif () | ||
| # end target build section |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Superfluous comment
| target_include_directories(lib_imgui | ||
| PUBLIC ${IMGUI_INCLUDE_DIRS} | ||
| PUBLIC "${CMAKE_CURRENT_LIST_DIR}/wrapper") | ||
| target_link_libraries(lib_imgui PRIVATE d3d8lib) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can add newline before this for readability.
| corei_libraries_include | ||
| resources | ||
| ) | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file does not need touching. Newline can be reverted.
| [["WWCommon.h"]] | ||
| ) | ||
|
|
||
| if (RTS_BUILD_OPTION_IMGUI) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of conditionally linking the library, consider creating a dummy library conditionally. This makes the cmake code simpler at the link sites.
There is an example with
if (IS_VS6_BUILD)
include(cmake/stlport.cmake)
else()
add_library(stlport INTERFACE)
endif()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps
if (RTS_BUILD_OPTION_IMGUI)
add_subdirectory(Source/ImGui)
#else
add_library(lib_imgui INTERFACE)
endif ()
| ShatterSystem::Init(); | ||
| TextureLoader::Init(); | ||
|
|
||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Revert superfluous edit.
| // calls ImGui::EndFrame() and ImGui::Render() but does NOT call | ||
| // ImGui::RenderDrawData(draw_data). The latter is not part of this wrapper | ||
| // by design (separation of concerns) and must be called explicitly. This is to be done | ||
| // when sending draw data off to the GPU to be drawn on screen, which in our case happens in DX8Wrapper::End_Scene() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment adds more confusion.
Can we design this in a way that is intuitive to use and does not need explanations?
Maybe have BeginFrame, EndFrame, RenderFrame?
Can EndFrame and RenderFrame not be done in one step?
Here is a minimal example of how an App can Render Imgui in an Update Loop:
// Setup Platform/Renderer backends
ImGui_ImplWin32_Init(hwnd);
ImGui_ImplDX9_Init(g_pd3dDevice);
// Main loop
while (true)
{
// Poll and handle messages (inputs, window resize, etc.)
// See the WndProc() function below for our to dispatch events to the Win32 backend.
MSG msg;
while (::PeekMessage(&msg, nullptr, 0U, 0U, PM_REMOVE))
{
::TranslateMessage(&msg);
::DispatchMessage(&msg);
if (msg.message == WM_QUIT)
m_app->prepare_shutdown_wait();
}
if (m_app->can_shutdown())
break;
// Handle lost D3D9 device
if (g_DeviceLost)
{
HRESULT hr = g_pd3dDevice->TestCooperativeLevel();
if (hr == D3DERR_DEVICELOST)
{
::Sleep(10);
continue;
}
if (hr == D3DERR_DEVICENOTRESET)
ResetDevice();
g_DeviceLost = false;
}
// Handle window resize (we don't resize directly in the WM_SIZE handler)
if (g_ResizeWidth != 0 && g_ResizeHeight != 0)
{
g_d3dpp.BackBufferWidth = g_ResizeWidth;
g_d3dpp.BackBufferHeight = g_ResizeHeight;
g_ResizeWidth = 0;
g_ResizeHeight = 0;
ResetDevice();
}
// Start the Dear ImGui frame
ImGui_ImplDX9_NewFrame();
ImGui_ImplWin32_NewFrame();
{
const ImGuiStatus error = m_app->update();
if (error != ImGuiStatus::Ok)
return error;
}
g_pd3dDevice->SetRenderState(D3DRS_ZENABLE, FALSE);
g_pd3dDevice->SetRenderState(D3DRS_ALPHABLENDENABLE, FALSE);
g_pd3dDevice->SetRenderState(D3DRS_SCISSORTESTENABLE, FALSE);
const ImVec4 clear_color = m_app->get_clear_color();
D3DCOLOR clear_col_dx = D3DCOLOR_RGBA(
(int)(clear_color.x * clear_color.w * 255.0f),
(int)(clear_color.y * clear_color.w * 255.0f),
(int)(clear_color.z * clear_color.w * 255.0f),
(int)(clear_color.w * 255.0f));
g_pd3dDevice->Clear(0, nullptr, D3DCLEAR_TARGET | D3DCLEAR_ZBUFFER, clear_col_dx, 1.0f, 0);
if (g_pd3dDevice->BeginScene() >= 0)
{
ImGui::Render();
ImGui_ImplDX9_RenderDrawData(ImGui::GetDrawData());
g_pd3dDevice->EndScene();
}
// Update and Render additional Platform Windows
const ImGuiIO &io = ImGui::GetIO();
if (io.ConfigFlags & ImGuiConfigFlags_ViewportsEnable)
{
ImGui::UpdatePlatformWindows();
ImGui::RenderPlatformWindowsDefault();
}
HRESULT result = g_pd3dDevice->Present(nullptr, nullptr, nullptr, nullptr);
if (result == D3DERR_DEVICELOST)
g_DeviceLost = true;
}
// Cleanup
ImGui_ImplDX9_Shutdown();
ImGui_ImplWin32_Shutdown();|
|
||
|
|
||
| // TheSuperHackers @feature jurassiclizard 16/01/2026 imgui integration (PR#2127) | ||
| // see details under WinMain.cpp (WndProc()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Superfluous comments
| ImGuiIO& io = ImGui::GetIO(); | ||
| io.ConfigFlags |= ImGuiConfigFlags_NavEnableKeyboard; | ||
| io.ConfigFlags |= ImGuiConfigFlags_NavEnableGamepad; | ||
| // Dark Style |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Superfluous comment
| #ifdef RTS_HAS_IMGUI | ||
| ImGui_ImplDX8_Shutdown(); | ||
| ImGui_ImplWin32_Shutdown(); | ||
| ImGui::DestroyContext(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it legal to legal to call all these if DX8Wrapper::Create_Device was never called? Is that possible?
|
|
||
| #ifdef RTS_HAS_IMGUI | ||
| if (ImGui_ImplWin32_WndProcHandler(hWnd, message, wParam, lParam)) { | ||
| return true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Better return integer. What is return 1 doing?


Note : to test this please configure in Debug mode before building with
-A Win32 ~~-DRTS_BUILD_OPTION_DEBUG=ON~~ -DRTS_BUILD_OPTION_IMGUI=Onall the code is gated behind a RTS_IMGUI_ENABLED perprocessor define for
debugging purposesWhat it does : Integrates the Dear Imgui framework and loads a console and demo window overlay
in debug mode(generalsv, generalszh, WorldBuilderV and WorldBuilderZH)What is missing
Some useful command or Ideas what it should look like: this PR will just focus on build and runtime integration via ImGui DemoCurrently it is permanently enabled when the game is builtwith the imgui option onGated behind RTS_BUILD_OPTION_IMGUICode cleanup and SuperHackers Comments etc...doneBackporting to Generalsdone